fix: use relative paths for external_directory permission patterns#108
Closed
ariane-emory wants to merge 8 commits intodevfrom
Closed
fix: use relative paths for external_directory permission patterns#108ariane-emory wants to merge 8 commits intodevfrom
ariane-emory wants to merge 8 commits intodevfrom
Conversation
This allows users to configure external_directory permissions using relative path patterns like "../sibling-project-*" instead of requiring absolute paths, making it consistent with how edit permissions work.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix: use relative paths for external_directory permission patterns
Summary
This PR fixes the
external_directorypermission to use relative paths instead of absolute paths, making it consistent with other permissions likeeditand allowing users to configure patterns like../sibling-project-*.Problem
The
external_directorypermission was using absolute paths internally, which meant user-configured relative path patterns would never match. For example:{ "permission": { "external_directory": { "*": "ask", "../sibling-project-*": "allow" } } }This config would not work because when accessing
/Users/foo/sibling-project-bar/file.txtfrom/Users/foo/myproject, the internal pattern would be/Users/foo/sibling-project-bar/*, which doesn't match../sibling-project-*.Solution
Changed
assertExternalDirectoryto compute relative paths usingpath.relative(Instance.directory, parentDir):Now when accessing
/Users/foo/sibling-project-bar/file.txtfrom/Users/foo/myproject, the pattern becomes../sibling-project-bar/*, which correctly matches../sibling-project-*.Rationale
Consistency: The
editpermission already uses relative paths (path.relative(Instance.worktree, filePath)), soexternal_directoryshould behave the same wayPrinciple of least surprise: The documentation shows relative path examples, and users reasonably expect them to work
Portability: Relative paths work across machines, while absolute paths are machine-specific
Minimal change: The fix is a single-line addition that converts the path before building the glob pattern
Changes
src/tool/external-directory.ts: Usepath.relative()to compute relative path patternstest/tool/external-directory.test.ts: Updated test expectations for relative pathstest/tool/read.test.ts: Updated test to check for relative path formatTesting
All existing tests pass with updated expectations:
test/tool/external-directory.test.ts- 5 teststest/tool/read.test.ts- 26 teststest/tool/bash.test.ts- 12 tests (unchanged)test/agent/agent.test.ts- 34 testsNote
The
bashtool has separateexternal_directorylogic that still uses absolute paths. This can be addressed in a follow-up PR to keep this change tightly scoped.